Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix function estimate_memory_usage() function #900

Merged
merged 2 commits into from
Sep 15, 2023

Conversation

drdzyk
Copy link
Contributor

@drdzyk drdzyk commented Sep 14, 2023

Currently estimate_memory_usage() function shows incorrect results for both vector and map internal storage.

It actually shows how much memory is needed for copying this array, rather than how much memory it currently takes. So it takes only size to calculate the memory amount and ignores capacity.
This logic is used in InstanceDeepCopyVisitor to check if it will be enough memory for copying beforehand. So in this PR this logic is extracted to separate function calculate_memory_for_copying.
And estimate_memory_usage for arrays now returns the correct size based on capacity.

@drdzyk drdzyk requested a review from DrDet September 14, 2023 11:24
@DrDet DrDet force-pushed the seismont/fix-estimate-memory-usage branch 2 times, most recently from 08e1ce1 to 967945c Compare September 15, 2023 11:53
@DrDet DrDet added this to the next milestone Sep 15, 2023
@DrDet DrDet added runtime Feature related to runtime small fix When it's not an huge enhancement labels Sep 15, 2023
@DrDet DrDet merged commit afdf709 into master Sep 15, 2023
5 checks passed
@DrDet DrDet deleted the seismont/fix-estimate-memory-usage branch September 15, 2023 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
runtime Feature related to runtime small fix When it's not an huge enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants